Make OverflowError more informative #22761
Conversation
|
looks mostly fine to me, no ideas on the separate from-C version |
|
Thanks for triggering nanosoldier.
Since this the last in the chain of PRs, I'm fine with leaving this open for a while to see if any objections arise. |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
base/checked.jl
Outdated
| @@ -90,16 +90,18 @@ The overflow protection may impose a perceptible performance penalty. | |||
| function checked_neg(x::T) where T<:Integer | |||
| checked_sub(T(0), x) | |||
| end | |||
| throw_overflowerr_negation(x) = (@_noinline_meta; | |||
| throw(OverflowError("cannot compute -x for x = $x::$(typeof(x))"))) | |||
There was a problem hiding this comment.
While you're at it, it would be useful to mention "checked" somewhere, so that it's clear this is not an error from the standard - function. Same for abs.
NEWS.md
Outdated
| [msg])` prints as `ERROR: DomainError with val:\nmsg`. ([#20005], | ||
| [#22751]) | ||
| "ERROR: InexactError: func(type, -3)", `DomainError(val, | ||
| [msg])` prints as "ERROR: DomainError with val:\nmsg", and `OverflowError(msg)` prints as "Error: OverflowError: msg". ([#20005], |
There was a problem hiding this comment.
Perhaps wrap this line at the comma?
There was a problem hiding this comment.
all 3 capitalize the ERROR don't they? or is there a discrepancy somehow?
|
@nanosoldier |
|
Something seems wrong with CI right now? |
|
bsd ci error is #23143 |
|
It's more than that, the Linux travis bots seem to be having trouble downloading UnicodeData.txt. |
|
That failure has been seen a lot recently on Travis I feel like, perhaps you could restart and hope for the best? 🤷♂️ |
|
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan |
|
Given staticfloat/cache.julialang.org#16, that should be hitting our cache server. I'm not sure why it didn't, or why that has been failing again. |
|
OK, that's a nice nanosoldier run. Not quite sure why I got performance enhancements, but I'll take it. (I manually checked the regressions in the first nanosoldier run; they were real, but I fixed them in this version, and perhaps there were collateral benefits.) Appveyor likes this and I can also verify tests pass on Linux locally. I'll try restarting the travis and BSD bots, but if they fail again I'm sorta inclined to hit merge anyway? |
|
👍 @travis-ci has been a sh*tshow since they decided to oversubscribe all their VMs, rendering their service completely and utterly useless to us despite the fact that we are paying customers 😡. |
src/init.c
Outdated
| @@ -785,7 +785,7 @@ void jl_get_builtin_hooks(void) | |||
| jl_errorexception_type = (jl_datatype_t*)core("ErrorException"); | |||
| jl_stackovf_exception = jl_new_struct_uninit((jl_datatype_t*)core("StackOverflowError")); | |||
| jl_diverror_exception = jl_new_struct_uninit((jl_datatype_t*)core("DivideError")); | |||
| jl_overflow_exception = jl_new_struct_uninit((jl_datatype_t*)core("OverflowError")); | |||
| jl_c_overflow_exception = jl_new_struct_uninit((jl_datatype_t*)core("COverflowError")); | |||
There was a problem hiding this comment.
This seems sort of odd to have two overflow exception types. It would be nicer to use something like:
jl_c_overflow_exception = core("overflow_error_from_c")const overflow_error_from_c = OverflowError(ErrorException(""))But there's only one active use of this in src (in jl_compute_field_offsets), so it's perhaps better just to delete this entirely and use jl_errorf there.
The last of the big 3 inscrutable error messages (InexactError, DomainError, and OverflowError). This one is a little messier than the other two because there's one C creator that I didn't easily see how to change; when I previously tried to throw a more sophisticated error message from C (#20005) it didn't go very well. So here I just took the expedient approach and created two distinct classes of Exceptions, one inscrutable one to be thrown from the C code, and a nice, easy-to-debug one from Julia.